Skip to content

fix(serilog): logs from application namespaces beginning with "Sentry" are discarded - #5456

Open
jamescrosswell wants to merge 2 commits into
mainfrom
serilog-context-filter-5265
Open

fix(serilog): logs from application namespaces beginning with "Sentry" are discarded#5456
jamescrosswell wants to merge 2 commits into
mainfrom
serilog-context-filter-5265

Conversation

@jamescrosswell

@jamescrosswell jamescrosswell commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Closes #5265

Summary

SentrySink discarded any Serilog event whose SourceContext started with "Sentry.". That heuristic dates back to 2019 ("don't log self log") and is meant to stop the SDK's own diagnostics looping back into Sentry — but it also matches application code. An app logging under, say, Sentry.Samples.AspNetCore.Serilog.Program had its log events dropped before anything was created: no event, no breadcrumb, no structured log, and therefore no event processors run. That last symptom is how #5265 was reported, but ISentryEventProcessor was never the problem.

Our own samples/Sentry.Samples.AspNetCore.Serilog uses that namespace, so the sample demonstrating the ILogger path was silently broken — which is where the reporter picked it up.

The filter can't simply be removed. MelDiagnosticLogger routes SDK diagnostics through ILogger<ISentryClient> whenever Debug is enabled (ApplicationBuilderExtensions.cs), and with Serilog as the MEL backend that's a genuine feedback loop. The isReentrant guard in the sink doesn't cover it, because the transport logs from the background worker thread.

So the check now matches the namespace roots the SDK actually owns, rather than a bare "Sentry." prefix.

Notes for review

  • The root list is generated, not hand-maintained. A GenerateSentrySdkNamespaces target in Sentry.csproj reads the package list out of .craft.yml at build time and emits SentrySdkNamespaces.g.cs. No reflection (AOT/trimming-safe) and no list to keep in sync as packages are added. .craft.yml's line order is fixed, so output is deterministic. The target hard-errors rather than emitting an empty list, since an empty list would silently restore the loop risk.
  • The core Sentry package can't be a prefix root. All of its types live under Sentry. — and so does user code — so treating it as a prefix would reproduce the original bug. Core has no logging dependency of its own, and the only category it ever logs under is Sentry.ISentryClient (from MelDiagnosticLogger), so that plus a bare Sentry are matched exactly. This is the one hardcoded string in the change; it's commented next to the thing that produces it.
  • Sentry.Extensions.Logging is included deliberately. SentryLogger.IsFromSentry() carried the same heuristic plus a #if DEBUG carve-out for Sentry.Samples., which only ever helped samples built from source — anyone on the released package was still affected. Both integrations now share one helper. Sentry.NLog and Sentry.Log4Net have never had this filter at all; that asymmetry is left as-is here.
  • .craft.yml gains Sentry.Maui.CommunityToolkit.Mvvm, which is packable but was missing from the registry list. Worth a second pair of eyes: it's a release-config change riding along with a bug fix, and it means the release registry starts tracking that package... main reason for including it in this PR is that we're using craft.yml to generate the list of namespaces that we match against for log filtering.

…ing with "Sentry"

The Serilog sink dropped any log event whose SourceContext started with "Sentry.",
a heuristic meant to keep the SDK's own diagnostics from looping back into Sentry.
It also matched application code, so an app logging under e.g.
"Sentry.Samples.AspNetCore.Serilog.Program" had its events silently discarded -
no event, no breadcrumb, no structured log, and no event processors run. Our own
ASP.NET Core Serilog sample is written in exactly that namespace.

The filter is still needed: MelDiagnosticLogger routes SDK diagnostics through
ILogger<ISentryClient> when Debug is enabled, which Serilog would otherwise feed
straight back in. So instead of matching a bare prefix, match the namespace roots
the SDK actually owns. Those are generated at build time from the package list in
.craft.yml, so the list can't go stale as packages are added.

The core "Sentry" package can't be a prefix root - all its types live under
"Sentry.", as does user code - so it is matched exactly, along with the one
category core ever logs under.

Sentry.Extensions.Logging shared the same heuristic, including a #if DEBUG
carve-out for "Sentry.Samples." that only ever helped samples built from source.
Both integrations now use the same helper.

Also adds the missing Sentry.Maui.CommunityToolkit.Mvvm entry to .craft.yml.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jamescrosswell jamescrosswell changed the title fix(serilog): stop discarding logs from application namespaces beginning with "Sentry" fix(serilog): logs from application namespaces beginning with "Sentry" are discarded Jul 29, 2026
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.70%. Comparing base (8d4d3f6) to head (252fe48).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5456   +/-   ##
=======================================
  Coverage   74.69%   74.70%           
=======================================
  Files         512      513    +1     
  Lines       18722    18729    +7     
  Branches     3660     3663    +3     
=======================================
+ Hits        13985    13992    +7     
  Misses       3865     3865           
  Partials      872      872           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread test/Sentry.Serilog.Tests/AspNetCoreIntegrationTests.cs Outdated
Comment thread test/Sentry.Serilog.Tests/AspNetCoreIntegrationTests.cs Outdated
Comment thread test/Sentry.Serilog.Tests/SentrySinkTests.cs Outdated
Comment thread test/Sentry.Tests/Internals/SentrySdkNamespacesTests.cs Outdated
Comment thread test/Sentry.Tests/Internals/SentrySdkNamespacesTests.cs Outdated
.
Co-authored-by: James Crosswell <jamescrosswell@users.noreply.github.com>
@jamescrosswell
jamescrosswell marked this pull request as ready for review July 30, 2026 04:14
@github-actions github-actions Bot added the risk: high PR risk score: high label Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: high PR risk score: high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sentry.Serilog Does Not Use ISentryEventProcessor When Microsoft.Extensions.Logging.ILogger Is Used

1 participant